Skip to content

feat(silo)!: support multiple lineage systems - #998

Merged
taepper merged 1 commit into
mainfrom
691-Support-multiple-lineage-systems
Oct 3, 2025
Merged

feat(silo)!: support multiple lineage systems#998
taepper merged 1 commit into
mainfrom
691-Support-multiple-lineage-systems

Conversation

@taepper

@taepper taepper commented Sep 26, 2025

Copy link
Copy Markdown
Collaborator

BREAKING CHANGE: Multiple breaking changes to lineage configuration:

  • PreprocessingConfig: lineageDefinitionsFilename renamed to lineageDefinitionFilenames and changed from string to array
  • DatabaseConfig: generateLineageIndex changed from boolean to string (expects lineage definition filename)

resolves #691

Summary

This adds the possibility to define multiple lineage definition systems.

In preprocessing, the lineage systems are given as a list of filenames in YAML. The filenames are expected as relative from inputDirectory.

The string columns with lineage indexes are still marked with generateLineageIndex in the database config. Instead of true, the value should be the name of the lineage definition file (.yaml may be omitted).

PR Checklist

  • All necessary documentation has been adapted or there is an issue to do so.
  • The implemented feature is covered by an appropriate test.

@github-actions

github-actions Bot commented Sep 26, 2025

Copy link
Copy Markdown
Contributor

This is a preview of the changelog of the next release. If this branch is not up-to-date with the current main branch, the changelog may not be accurate. Rebase your branch on the main branch to get the most accurate changelog.

Note that this might contain changes that are on main, but not yet released.

Changelog:

0.9.0 (2025-10-02)

⚠ BREAKING CHANGES

  • silo: Multiple breaking changes to lineage configuration:
    • PreprocessingConfig: lineageDefinitionsFilename renamed to
      lineageDefinitionFilenames and changed from string to list
    • DatabaseConfig: generateLineageIndex changed from boolean to string
      (expects lineage definition filename)

Features

  • silo: add a health endpoint that checks whether the api is able to answer requests (5422ff1)
  • silo: remove TBB dependency (#985) (91ec897), closes #873
  • silo: support multiple lineage systems (3908395)
  • update arrow to version 21.0.0 (478366e)

Bug Fixes

  • test: properly free request handlers in tests (0f9e0d9)

@taepper
taepper force-pushed the 691-Support-multiple-lineage-systems branch from c881612 to 8491418 Compare September 26, 2025 09:37
@taepper
taepper force-pushed the 691-Support-multiple-lineage-systems branch from 8491418 to db2709e Compare September 26, 2025 09:37
@taepper
taepper requested a review from anna-parker September 26, 2025 09:38
@taepper
taepper force-pushed the 691-Support-multiple-lineage-systems branch from db2709e to 9e74c8c Compare September 26, 2025 10:15
@fengelniederhammer

fengelniederhammer commented Sep 29, 2025

Copy link
Copy Markdown
Contributor

Multiple breaking changes to lineage configuration:

* PreprocessingConfig: `lineageDefinitionsFilename` renamed to `lineageDefinitionFilenames` and changed from string to array

* DatabaseConfig: `generateLineageIndex` changed from boolean to string (expects lineage filename with automatic prefix/suffix detection)

Would it make sense to change lineageDefinitionFilenames to an object? The values would be the lineage file and generateLineageIndex could then reference the keys there (instead of the filename with a autocomplete attempt that may be confusing and error prone).

# preprocessing config
lineageDefinitionFilenames:
  pangoLineage: 'my-pango-lineages.yaml'
  nextstrainClade: 'my-nextstrain-clades.yaml'
---
# db config
  metadata:
    - name: myPangoLineageField
      generateLineageInedx: pangoLineage

Edit: if I call my lineage file e.g. "lineage.yaml", would generateLineageInedx: '' be valid in the config (with the current implementation)?

Comment thread src/silo/config/initialize_config.cpp Outdated
Comment thread src/silo/initialize/initializer.cpp Outdated
Comment thread src/config/source/yaml_file.cpp Outdated
@taepper

taepper commented Sep 29, 2025

Copy link
Copy Markdown
Collaborator Author

Would it make sense to change lineageDefinitionFilenames to an object? The values would be the lineage file and generateLineageIndex could then reference the keys there (instead of the filename with a autocomplete attempt that may be confusing and error prone).

Unfortunately, this is not possible, as the preprocessing-config can also be supplied by CLI arguments / env vars, hence, it can only be a string. -> only partially true, we introduced a list type parameter instead, see later discussion

Mid-term I do not think this matters too much, as the current preprocessing should be replaced by a more idiomatic and dynamic API for schema management

@fengelniederhammer

Copy link
Copy Markdown
Contributor

Would it make sense to change lineageDefinitionFilenames to an object? The values would be the lineage file and generateLineageIndex could then reference the keys there (instead of the filename with a autocomplete attempt that may be confusing and error prone).

Unfortunately, this is not possible, as the preprocessing-config can also be supplied by CLI arguments / env vars, hence, it can only be a string.

Mid-term I do not think this matters too much, as the current preprocessing should be replaced by a more idiomatic and dynamic API for schema management

Ok that is unfortunate. I could imagine that we could still get keys from CLI arguments by implementing a syntax like --option 'key:value' but I'm not sure whether that's worth the trouble.

But I'd still like to get rid of the heuristics that try to autocomplete the filename. Do we need that at all? The DB config is written once and it doesn't really matter how long the config values are. We could simply require maintainers to put the whole filename there (maybe without the .yaml ending). That would be a lot more transparent, easier to explain and less error prone. IMO wouldn't lose anything.

@taepper
taepper force-pushed the 691-Support-multiple-lineage-systems branch from 76e4f68 to 7eb9d61 Compare October 1, 2025 08:20

@fengelniederhammer fengelniederhammer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment thread src/silo/config/preprocessing_config.cpp
@taepper
taepper force-pushed the 691-Support-multiple-lineage-systems branch from 53064a1 to 27ff74c Compare October 2, 2025 11:35
BREAKING CHANGE: Multiple breaking changes to lineage configuration:
- PreprocessingConfig: `lineageDefinitionsFilename` renamed to
  `lineageDefinitionFilenames` and changed from string to list
- DatabaseConfig: `generateLineageIndex` changed from boolean to string
  (expects lineage definition filename)
@taepper

taepper commented Oct 2, 2025

Copy link
Copy Markdown
Collaborator Author

Ok that is unfortunate. I could imagine that we could still get keys from CLI arguments by implementing a syntax like --option 'key:value' but I'm not sure whether that's worth the trouble.

We decided to change the code to store the parameter as a list directly, but CLI arguments and env vars will currently throw errors, when trying to define a list-type parameter (e.g. lineageDefinitionFilenames).

The follow-up issue #1010 has been added

@taepper

taepper commented Oct 2, 2025

Copy link
Copy Markdown
Collaborator Author

We might wait with merging this (and thus needing to bump SILO to 0.9), until #1007 is fixed

@taepper
taepper merged commit f4be635 into main Oct 3, 2025
9 checks passed
@taepper
taepper deleted the 691-Support-multiple-lineage-systems branch October 3, 2025 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support multiple lineage systems

2 participants